Skip to content

ci(preview): bisect invalid seed rows instead of replaying batches row-by-row (stage 3) - #1151

Draft
kyle-compute wants to merge 1 commit into
ci/preview-fencingfrom
ci/preview-seed-loader
Draft

ci(preview): bisect invalid seed rows instead of replaying batches row-by-row (stage 3)#1151
kyle-compute wants to merge 1 commit into
ci/preview-fencingfrom
ci/preview-seed-loader

Conversation

@kyle-compute

@kyle-compute kyle-compute commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Why this PR exists

  • Preview seed run fix: preserve verdict state and skip cancelled preview gates #1139 spent 318 seconds loading sampled production data.
  • One constraint-violating row caused an entire approximately 900-row batch to fall back to one SQL statement per row.
  • Five bad legacy rows could therefore create thousands of connection-pool round trips.
  • The fallback caught broad DBAPIError exceptions, so a dead connection, timeout, or serialization failure could be mislabeled as "bad data" and silently skipped.

Evidence

  • Old behavior for one invalid row in a 1,000-row batch: one failed batch plus 1,000 row retries.
  • New divide-and-conquer bound for one invalid row: at most 2 × ceil(log2(n)) + 1 probes.
  • End-to-end smoke test sampled 36 rows, isolated a real unique-constraint collision, completed in approximately 0.5 seconds, and emitted the report.
  • Migration-before-seed correctness is already enforced elsewhere by the preview bootstrap fingerprint and orchestration tests; this PR does not duplicate that work.

What this PR changes

  • Replaces row-by-row fallback with recursive batch bisection.
  • Rolls back only the failed savepoint, splits the batch, and recurses only into failing halves.
  • Splits only IntegrityError / SQLSTATE class 23 failures.
  • Lets network, timeout, deadlock, and serialization errors fail the seed loudly.
  • Reports attempted batches, splits, skipped rows, SQLSTATE/constraint causes, stable row identifiers, and before/after table counts.
  • Sends the report through the existing preview timing artifact as seed telemetry.
  • Repairs heterogeneous test fixtures whose sparse dictionary keys had drifted from SQLAlchemy's multi-row insert behavior.

What this PR does not change

  • It does not relax a database constraint.
  • It does not skip a migration.
  • It does not change migration/seed ordering.
  • It does not treat transient infrastructure errors as skippable data.
  • It does not guarantee a fast seed when every row is invalid; that genuine worst case remains 2n - 1 probes.

Dependency and merge order

Validation completed

  • 104 loader and preview-seed tests passed.
  • The one-invalid-row, five-invalid-row, all-valid, all-invalid, composite-key, deterministic-report, and transient-error cases are covered.
  • Existing bootstrap ordering tests passed: 21 tests.
  • The DB-backed suite proves valid children land and invalid parent/child rows are reported with their constraint identities.

Remaining acceptance gate

  • Run this branch's preview against the production-sampled legacy fixture that reproduced fix: preserve verdict state and skip cancelled preview gates #1139.
  • Record total seed time, statements/probes, rows skipped, causes, and connection-pool behavior.
  • Required result: under 60 seconds, invalid rows visible, no constraint relaxation, and transient failures fatal.

Risk

  • A transient database failure that was previously hidden will now make preview red. That is intentional.
  • Two aggregate count round trips are added for telemetry.
  • A data set where nearly every row is invalid remains expensive and should be fixed at the source.

Rollback

Revert this PR to restore the old loader. The seed telemetry field is additive and consumers already tolerate its absence. Reverting also restores the unsafe broad fallback, so rollback should be temporary while the concrete failure is fixed.

…w-by-row

The #1139 seed pathology: one constraint-violating row in a multi-row
upsert batch dropped the whole chunk into a row-by-row fallback, so five
legacy rows among ~900-row chunks cost thousands of pooler round trips
(318s). The fallback now rolls the failed batch back at its savepoint,
splits it in half, and recurses only into failing halves until invalid
rows stand alone: O(k log n) probes for k bad rows in n rows, with valid
rows retained.

Only IntegrityError (SQLSTATE class 23) is splittable. The old catch-all
also swallowed DBAPIError, so a dead connection or serialization failure
was silently recorded as skipped rows -- transient infrastructure errors
now abort the load instead of masquerading as bad data.

Every seed run emits a deterministic report (batches attempted/split,
rows skipped, SQLSTATE + constraint names with table-qualified row ids,
before/after row counts by table) that flows through a new prepare-job
output into the timing artifact and the job summary.

Also repairs the local seed-test fixtures, which had silently rotted
(they skip without ODDISH_DATABASE_URL): SQLAlchemy compiles multi-row
inserts from the first dict's keys, so sparse rows had extra keys
silently dropped and missing keys hard-error; a _rows() helper now
unions the keys of each heterogeneous fixture group.
@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
oddish-app Ready Ready Preview Aug 11, 2026 12:56am

@github-actions

Copy link
Copy Markdown
Contributor

Oddish preview

Commit: 4c1c5565b948656f6d795ab367dfbe01531e76a6

Surface Link Target
Frontend https://pr-1151.oddish.app Vercel preview for 4c1c556
Backend oddish-pr-1151 oddish-pr-1151
Database project lkzbtctcircfrhrurhak project lkzbtctcircfrhrurhak

Vercel deployment URL: https://oddish-meg535pol.oddish.app

Plan:

  • Frontend deploy: true
  • Backend deploy: true
  • Migrations: false

This comment is updated by the PR Preview workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant